PR Review: Comprehensive Test Suite

Great addition overall — 176 tests across 6 files with solid coverage of security-sensitive paths (SSRF, path traversal, header redaction). A few things worth addressing:

Issues

1. os.Chdir in setupWorkDir is not parallel-safe (cmd/request_test.go)

os.Chdir is process-wide. If any test using setupWorkDir ever gets t.Parallel() added, it will cause a data race. Consider passing the directory explicitly to functions under test instead, or using t.Chdir() (Go 1.24+) which is cleanup-safe and parallel-aware.

// Better: use t.Chdir (Go 1.24+) instead of rolling your own
func TestReadBodyFromFile_ValidFile(t *testing.T) {
    dir := t.TempDir()
    t.Chdir(dir)
    // ...
}

2. import_time violates Go naming conventions (cmd/export_test.go:~388)

Go uses camelCase for local variables — import_time should be req or histReq.

// Current
import_time := model.Request{...}
store.AddToHistory(import_time)

// Fix
req := model.Request{...}
store.AddToHistory(req)

3. warnIfSensitiveBody tests only assert no-panic, not behavior (cmd/request_test.go)

These tests pass even if the warning logic is accidentally removed:

func TestWarnIfSensitiveBody_ContainsPassword(t *testing.T) {
    warnIfSensitiveBody(`{"username":"alice","password":"s3cr3t"}`)
    // ← no assertion that a warning was actually emitted
}

If warnIfSensitiveBody writes to stderr or returns a boolean, capture and assert it. If it's a pure side-effect (prints a warning), consider refactoring to accept an io.Writer so tests can verify output. As-is, these tests guard against panics but not regressions in the actual warning behavior.

4. New transitive dependencies in go.mod are substantial

The modernc.org/* packages (libc, gc, mathutil, memory, strutil, token) are the CGo-free SQLite implementation — 7 new indirect dependencies totalling significant binary size. Confirm this was already a transitive dep of your SQLite driver and just wasn't pinned before, rather than a newly introduced dependency. If it's new, it should be explicit in the PR description.

Minor Notes

• TestIsPrivateOrReservedHost doesn't cover IPv6 private ranges (::1, fd00::/8, fe80::/10). If validateURL accepts IPv6, these should be tested.
• TestConvertPostmanRequest_BodyModeCaseInsensitive tests "RAW" (uppercase). Worth a quick check that the production code uses strings.EqualFold or similar rather than just == "raw" — the test title implies case-insensitivity is intentional behavior.
• None of the tests use t.Parallel(). For a 176-test suite this is fine, but flagging it as a future optimization opportunity once the os.Chdir issue above is resolved.

What's Working Well

• Security test coverage is thorough: SSRF (AWS/GCP/Alibaba/ECS metadata endpoints), path traversal (../../../etc/passwd, absolute paths), header redaction (case-insensitive, all sensitive header types)
• t.Setenv("HOME", t.TempDir()) for storage isolation is correct and clean
• Error message assertions (not just err != nil) make failures much more debuggable
• Section comments with box-drawing characters make the test files very readable
• TestBuildPostmanFromCollection_NotFound asserting "not found" in the error message is a good regression guard

Overall a solid test suite — address the warnIfSensitiveBody assertions and the naming nit before merging.